Summary

P0 robustness items surfaced by a structured two-voice brainstorm with codex (gpt-5-codex). Two passes: codex critiqued my opening positions, I pushed back on three of codex's downgrades, both sides did file-grounded verification of every claim. Of the original 4 P0 items, 2 ship here and 2 are deferred with documented rationale (the dialogue revealed they were based on flawed mental models of the actual code).

Ships

• Pipeline orphan FK rows — new Repository.BatchCreateAll persists Trace→Span→Log inside a single db.Transaction. A panic between two BatchCreate* calls used to leak orphan parent rows; now the entire batch rolls back atomically. Trace insert preserves the existing ON CONFLICT DO NOTHING idempotency, so DLQ replay re-attempts cleanly.
• DLQ Enqueue f.Sync() — adds fsync before close so a host crash between Write and Close can't leave a torn JSON file that permanently consumes a retry slot until DLQ_MAX_RETRIES evicts it.

Behavior change worth a second look

TestPipeline_FailedTracesContinuesToSpans is renamed/rewritten as TestPipeline_FailedTracesAbortsBatch. Trace insert errors now abort the batch instead of being "tolerated". The previous tolerance was a fragile workaround — traces are idempotent on retry, so a DLQ replay re-attempts cleanly. TestPipeline_FailedSpansSkipsLogs (orphan-log invariant) preserved.

Deferred (documented, not silent)

• DLQ replay poisoned-pill via ON CONFLICT DO NOTHING — Span, Log, MetricBucket have no unique-index beyond auto-increment id, so the clause is a no-op there. Real fix needs a schema migration (uniqueIndex on (tenant_id, trace_id, span_id) for spans, plus a pre-migration dedup on existing rows). Out of scope for P0; needs its own design pass.
• GraphRAG refresh shadow-swap — codex framed this as "multi-second write-lock stall every 60s". Code reading shows rebuildFromDBForTenant does many tiny Upsert* calls (microseconds each, lock briefly per call), not a single bulk write-lock. Contention is distributed, not bursty. Demoted to a P2 optimization, deferred to a future PR if metrics show it's needed.

Skipped at the brainstorm stage (verification rejected each)

• MCP semaphore-on-panic (PR fix(post-review): H1-H4 + C1 from deep code review #56's defer release() inside runWithTimeout already covers it)
• Drain template startup race (reload is synchronous in New() before Start() spawns workers)
• WS slow-client (per-client buffer is 256 + non-blocking eviction in flush())
• MCP cache invalidation (5s TTL is by-design)
• DB query deadline at repo layer (ctx already propagates; gap is at handler middleware)
• GraphRAG TraceStore restart loss (DB is source of truth; operability only)

Test plan

• go vet ./... clean
• go build ./... succeeds
• go test ./... -race -count=1 — 404 tests pass across 27 packages (pre-existing flake on TestMCP_TenantIsolation_AllGraphRAGTools under load surfaced once, passed on rerun and in isolation; flagged as a follow-up to investigate)
• TestPipeline_FailedTracesAbortsBatch and TestPipeline_FailedSpansSkipsLogs both pass under -race
• commit signed (ED25519)

🤖 Generated with Claude Code